Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dasherized-test-name-generator-option RFC #216

Closed
wants to merge 2 commits into from

Conversation

GabrielCW
Copy link

Test name examples for `some-multi-word-service-test.js` :

* With `--dasherize`: `Unit | Service | some-multi-word-service`
* With `--humanize` (default today): `Unit | Service | some multi word service`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this RFC should just set the new default to dasherize, but without adding any new options that would increase the complexity

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best case scenario for me ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this RFC should only be about changing the defaults. I am generally 👎 on adding new options...

@cibernox
Copy link
Contributor

I also agree that humanized names make no sense. I'm all in to change it.

@rwjblue
Copy link
Member

rwjblue commented Mar 20, 2017

Heh, I actually think we should remove the description completely... But if we can't do that, we should use dasherized version.

@GabrielCW
Copy link
Author

According to your feedback, I updated the RFC by removing the options.

The fix is also simpler, although I don't know the uses of ember-cli-test-info well enough to know if the humanize() function is called elsewhere, as it is public.

@Turbo87
Copy link
Member

Turbo87 commented Apr 13, 2017

Heh, I actually think we should remove the description completely... But if we can't do that, we should use dasherized version.

@rwjblue if we pull in emberjs/ember-qunit#258 then removing the test description is not an option

can we do anything to move this forward? I'm constantly being trolled by this when trying to test only single components...

@rwjblue
Copy link
Member

rwjblue commented Apr 13, 2017

@rwjblue if we pull in emberjs/ember-qunit#258 then removing the test description is not an option

I think you misunderstand me, I am saying we do not need both a name and a description. We should only have one, and actually it matches better with emberjs/ember-qunit#258!!

@Turbo87
Copy link
Member

Turbo87 commented Apr 13, 2017

@rwjblue indeed, I still don't understand what you mean. what are your definitions of "name" and "description"?

@rwjblue
Copy link
Member

rwjblue commented Jan 21, 2018

@GabrielCW - Thank you very much for taking the time to write up this RFC!

I think it's very clearly correct, and in our opinion points out some pretty bad developer ergonomics in the current default generation system. It has been the policy for quite some time in Ember-land to consider bad developer ergonomics to be bugs.

I am going to close this RFC as unneeded and advocate for landing the changes proposed in this RFC as a straight up bug fix.

Thank you again for helping push this forward!

@rwjblue rwjblue closed this Jan 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants